Skip to content

(AI Generated) Keep MusicXML arpeggios with the same number apart - #2041

Merged
mscuthbert merged 1 commit into
cuthbertLab:masterfrom
rdebroiz:fix/arpeggio-spanner-per-offset
Oct 1, 2026
Merged

(AI Generated) Keep MusicXML arpeggios with the same number apart#2041
mscuthbert merged 1 commit into
cuthbertLab:masterfrom
rdebroiz:fix/arpeggio-spanner-per-offset

Conversation

@rdebroiz

@rdebroiz rdebroiz commented Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Fixes #2039: MusicXML import merges every <arpeggiate> sharing a number into one spanner, so MuseScore files (always number="1") end up with a single arpeggio per part.

The importer now matches an ArpeggioMarkSpanner on number and offset. Other spanners close on type="stop", so only arpeggios were affected. Version bumped to 11.0.0b10 since cached parses are wrong.

AI-assisted (Claude)

@rdebroiz
rdebroiz force-pushed the fix/arpeggio-spanner-per-offset branch from 631382e to 5f9405e Compare September 22, 2026 08:23
@coveralls

coveralls commented Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

coverage: 93.36% (+0.003%) from 93.357% — rdebroiz:fix/arpeggio-spanner-per-offset into cuthbertLab:master

@rdebroiz
rdebroiz force-pushed the fix/arpeggio-spanner-per-offset branch 4 times, most recently from 93055a3 to 896d367 Compare September 22, 2026 08:30
mscuthbert
mscuthbert requested changes Sep 29, 2026
View reviewed changes

@mscuthbert mscuthbert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need regression tests -- and why are we going to parent.parent ? Can arpeggiospanners span different parts or just different staves?

René-Paul Debroize
Keep MusicXML arpeggios with the same number apart
dd8a885 
number only tells apart simultaneous arpeggios within a part, so match
ArpeggioMarkSpanners on number and offset, per part, instead of reusing
the incomplete one. Version bumped to 11.0.0b10 to invalidate cached
parses. Fixes cuthbertLab#2039.
@rdebroiz

rdebroiz commented Sep 30, 2026

Copy link
Copy Markdown
Author

Added testArpeggioMarkSpannersSameNumberDifferentOffsets (the case from #2039) and testArpeggioMarkSpannersSameNumberDifferentParts; both fail on master.

The re-exported MusicXML renders the same either way, since <arpeggiate> is written on every note, but it matters to us: the merged spanner shows up when we diff scores with musicdiff, which works on music21's objects.

On parent.parent: you're right, the intent was to cover several voices, not several parts. The dict is now on the PartParser, so arpeggios still join across voices and staves of one part, never across parts.

@rdebroiz
rdebroiz force-pushed the fix/arpeggio-spanner-per-offset branch from 393b4ca to dd8a885 Compare September 30, 2026 08:55
@mscuthbert
mscuthbert self-requested a review October 1, 2026 20:20
mscuthbert
mscuthbert approved these changes Oct 1, 2026
View reviewed changes

@mscuthbert mscuthbert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for the change!

@mscuthbert
mscuthbert merged commit 55d2d4c into cuthbertLab:master Oct 1, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mscuthbert mscuthbert mscuthbert approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MusicXML import merges every <arpeggiate number="1"> of a part into one ArpeggioMarkSpanner

3 participants

@rdebroiz @coveralls @mscuthbert